docs: note the CORS requirement for CDN-served assets - #352
Conversation
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an upgrade-guide warning for deployments that serve frontend assets from a separate CDN origin, noting that the ES module move requires CORS headers to allow the site origin.
Changes:
- Adds a new caution/admonition block explaining the ES module + CORS requirement for CDN-served assets
- Clarifies that same-origin (non-CDN) deployments are unaffected
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Both review points addressed in aab64e0: the requirement is now anchored to its source (the frontend migration PR, linked in place of the versionless phrasing, so a concrete version can be substituted at release time), and origin is defined precisely as scheme, host, and port with an example. |
robinv8
left a comment
robinv8
left a comment
There was a problem hiding this comment.
Thanks for documenting this — the CORS requirement for CDN-served ES modules is real and worth calling out.
One concern about placement: docs/getting-started/upgrade.md is the generic upgrade path (Docker pull / binary upgrade / force upgrade). Most readers here are on same-origin deployments and never touch a CDN. A full :::caution block about CDN CORS is deployment/plugin configuration, not an upgrade procedure, so it feels out of place in this guide.
I'd suggest:
- Put the substantive note next to where CDN is already documented — e.g.
docs/getting-started/configfile.md(ui.public_url/ static assets), and/or thecdn-aliyun/cdn-s3plugin READMEs (which already need the concrete CORS rules). - If the upgrade guide should mention it at all, keep it to a short cross-link, e.g. “If you serve static assets from a separate CDN origin, ensure CORS is configured — see Config file / the CDN plugin README.” rather than the full caution block.
Happy to discuss if you had a specific reason to anchor it on the upgrade page (e.g. release-note visibility for operators who only re-read upgrade docs).
Adds a caution block to the upgrade guide for deployments that serve static assets from a separate CDN origin: with the frontend moving to ES modules in apache/answer#1567, the CDN must send Access-Control-Allow-Origin for the site origin or pages load without JavaScript. Same-origin deployments are unaffected and the note says so explicitly. Requested by review on that PR; wording carries the crossorigin rationale documented there. Companion to apache/answer#1567, best merged with or after it.